Conversation
0bffac0 to
c67e0a7
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate internal usage of overlay “outlet”/IgxOverlayOutletDirective patterns (now deprecated) across Ignite UI for Angular components, shifting overlays to rely on their natural DOM placement / updated overlay behavior. It also updates related tests and docs accordingly.
Changes:
- Removed internal
[outlet]/outlet: .../[igxToggleOutlet]usage across pickers, grids, query builder, snackbar, tooltip/toggle, and samples. - Introduced a dedicated
dragGhostHostelement on grids and updated column-moving templates to use it instead ofgrid.outlet. - Updated multiple unit/integration tests and READMEs to align with the new overlay behavior and DOM structure.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/drop-down/drop-down.sample.html | Removes deprecated [igxToggleOutlet] usage from sample. |
| projects/igniteui-angular/time-picker/src/time-picker/time-picker.component.ts | Removes outlet from default overlay settings. |
| projects/igniteui-angular/snackbar/src/snackbar/snackbar.component.ts | Stops selecting strategy based on outlet; uses positioning only. |
| projects/igniteui-angular/query-builder/src/query-builder/query-builder.component.spec.ts | Updates DOM assertions and dropdown querying to reflect new overlay placement. |
| projects/igniteui-angular/query-builder/src/query-builder/query-builder-tree.component.ts | Removes overlay outlet directive usage and related overlay settings wiring. |
| projects/igniteui-angular/query-builder/src/query-builder/query-builder-tree.component.html | Removes internal overlay outlet container and picker [outlet] bindings. |
| projects/igniteui-angular/query-builder/src/query-builder/query-builder-functions.spec.ts | Updates helper functions to find visible dropdowns without relying on an outlet container. |
| projects/igniteui-angular/grids/tree-grid/src/tree-grid.component.html | Removes row-editing outlet and adds #igxDragGhostHost. |
| projects/igniteui-angular/grids/tree-grid/src/tree-grid-integration.spec.ts | Adjusts button selection indices due to DOM structure changes. |
| projects/igniteui-angular/grids/tree-grid/src/tree-cell.component.html | Removes [outlet] / [igxToggleOutlet] bindings from cell editors/tooltips. |
| projects/igniteui-angular/grids/pivot-grid/src/pivot-row-dimension-header-group.component.html | Uses grid.dragGhostHost instead of grid.outlet for column moving ghost host. |
| projects/igniteui-angular/grids/pivot-grid/src/pivot-grid.component.html | Adds #igxDragGhostHost. |
| projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid.component.ts | Removes parent-row outlet override used for row-editing overlay routing. |
| projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid.component.html | Removes row-editing outlet and adds #igxDragGhostHost. |
| projects/igniteui-angular/grids/grid/src/grid.component.spec.ts | Updates loading overlay assertions to match new template structure. |
| projects/igniteui-angular/grids/grid/src/grid.component.html | Refactors loading overlay markup; adds #igxDragGhostHost. |
| projects/igniteui-angular/grids/grid/src/grid-row-selection.spec.ts | Fixes row selection assertion to use a stable row reference. |
| projects/igniteui-angular/grids/grid/src/grid-filtering-ui.spec.ts | Updates calendar lookups to not depend on grid outlet container. |
| projects/igniteui-angular/grids/grid/src/grid-cell-editing.spec.ts | Adjusts scrolling-based expectations for picker overlay dismissal. |
| projects/igniteui-angular/grids/grid/src/grid-base.directive.ts | Removes row-editing/loader outlet wiring; introduces dragGhostHost; updates overlay opening to avoid outlet usage. |
| projects/igniteui-angular/grids/grid/src/expandable-cell.component.html | Removes [outlet] / [igxToggleOutlet] bindings from expandable cell editors/tooltips. |
| projects/igniteui-angular/grids/grid/src/column-pinning.spec.ts | Updates assertions to avoid relying on overlay outlet container. |
| projects/igniteui-angular/grids/core/src/toolbar/grid-toolbar.base.ts | Removes outlet: this.grid.outlet from toolbar overlay toggling. |
| projects/igniteui-angular/grids/core/src/headers/grid-header-group.component.html | Uses grid.dragGhostHost for column moving ghost host. |
| projects/igniteui-angular/grids/core/src/filtering/excel-style/excel-style-date-expression.component.html | Removes picker [outlet] bindings. |
| projects/igniteui-angular/grids/core/src/filtering/base/grid-filtering-row.component.html | Removes picker [outlet] bindings. |
| projects/igniteui-angular/grids/core/src/common/grid.interface.ts | Adds dragGhostHost to the grid interface. |
| projects/igniteui-angular/grids/core/src/cell.component.ts | Removes outlet from overlay settings for cell editors. |
| projects/igniteui-angular/grids/core/src/cell.component.html | Removes [outlet] / [igxToggleOutlet] bindings from editors/tooltips. |
| projects/igniteui-angular/expansion-panel/src/expansion-panel/expansion-panel.spec.ts | Updates expected DOM child count after grid template changes. |
| projects/igniteui-angular/directives/src/directives/tooltip/tooltip-target.directive.ts | Removes outlet propagation logic from tooltip action flow. |
| projects/igniteui-angular/directives/src/directives/toggle/toggle.directive.ts | Removes deprecated igxToggleOutlet input and outlet wiring in click handler. |
| projects/igniteui-angular/directives/src/directives/toggle/toggle.directive.spec.ts | Removes tests and test component relying on igxToggleOutlet. |
| projects/igniteui-angular/directives/src/directives/toggle/README.md | Removes documentation for igxToggleOutlet and IgxOverlayOutlet usage. |
| projects/igniteui-angular/directives/src/directives/notification/notifications.directive.ts | Removes deprecated outlet input and outlet usage in overlay settings. |
| projects/igniteui-angular/date-picker/src/date-range-picker/README.md | Removes deprecated outlet input from docs. |
| projects/igniteui-angular/date-picker/src/date-range-picker/date-range-picker.component.ts | Removes deprecated outlet input from the component API. |
| projects/igniteui-angular/date-picker/src/date-picker/README.md | Removes deprecated outlet input from docs. |
| projects/igniteui-angular/date-picker/src/date-picker/picker-base.directive.ts | Removes deprecated outlet input from the shared picker base. |
| projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.ts | Removes deprecated outlet override and outlet-based overlay wiring. |
| projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.spec.ts | Updates tests to stop asserting/using the removed outlet input. |
Comments suppressed due to low confidence (1)
projects/igniteui-angular/directives/src/directives/toggle/toggle.directive.ts:470
- This change removes the deprecated
igxToggleOutletinput fromIgxToggleActionDirective, which is a public API surface (and previously documented). That’s a breaking change and doesn’t match the PR description (“internal code base”) / checklist (breaking changes & migrations unchecked). If the intent is API removal, please add a changelog entry and provide anng updatemigration (or keep the input as a no-op until the next major).
export class IgxToggleActionDirective implements OnInit {
protected element = inject(ElementRef);
protected navigationService = inject(IgxNavigationService, { optional: true });
/**
* Provide settings that control the toggle overlay positioning, interaction and scroll behavior.
* ```typescript
* const settings: OverlaySettings = {
* closeOnOutsideClick: false,
* modal: false
* }
* ```
* ---
* ```html
* <!--set-->
* <div igxToggleAction [overlaySettings]="settings"></div>
* ```
*/
@Input()
public overlaySettings: OverlaySettings;
/**
* @hidden
*/
@Input('igxToggleAction')
public set target(target: any) {
if (target !== null && target !== '') {
this._target = target;
}
}
/**
* @hidden
*/
public get target(): any {
if (typeof this._target === 'string') {
return this.navigationService.get(this._target);
}
return this._target;
}
protected _overlayDefaults: OverlaySettings;
protected _target: IToggleView | string;
/**
* @hidden
*/
@HostListener('click')
public onClick() {
const clonedSettings = Object.assign({}, this._overlayDefaults, this.overlaySettings);
this.updateOverlaySettings(clonedSettings);
this.target.toggle(clonedSettings);
}
projects/igniteui-angular/date-picker/src/date-picker/picker-base.directive.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
projects/igniteui-angular/directives/src/directives/toggle/toggle.directive.ts:470
- This PR is scoped as “Do not use outlet internally”, but this change removes the public
igxToggleOutletinput entirely. Even if it was deprecated, this is an API-breaking change for consumers still using it and should be treated as such (mark PR as breaking, add migration/changelog guidance, or keep the input as a deprecated no-op/alias to the new recommended setting).
export class IgxToggleActionDirective implements OnInit {
protected element = inject(ElementRef);
protected navigationService = inject(IgxNavigationService, { optional: true });
/**
* Provide settings that control the toggle overlay positioning, interaction and scroll behavior.
* ```typescript
* const settings: OverlaySettings = {
* closeOnOutsideClick: false,
* modal: false
* }
* ```
* ---
* ```html
* <!--set-->
* <div igxToggleAction [overlaySettings]="settings"></div>
* ```
*/
@Input()
public overlaySettings: OverlaySettings;
/**
* @hidden
*/
@Input('igxToggleAction')
public set target(target: any) {
if (target !== null && target !== '') {
this._target = target;
}
}
/**
* @hidden
*/
public get target(): any {
if (typeof this._target === 'string') {
return this.navigationService.get(this._target);
}
return this._target;
}
protected _overlayDefaults: OverlaySettings;
protected _target: IToggleView | string;
/**
* @hidden
*/
@HostListener('click')
public onClick() {
const clonedSettings = Object.assign({}, this._overlayDefaults, this.overlaySettings);
this.updateOverlaySettings(clonedSettings);
this.target.toggle(clonedSettings);
}
projects/igniteui-angular/directives/src/directives/notification/notifications.directive.ts:90
IgxNotificationsDirectivepreviously exposed anoutletinput (deprecated) and included it in the overlay settings; this change removes the input and behavior. If the intent is only to stop using outlets internally, this is a breaking public API/behavior change and should be reflected as such (breaking-change flag + migration), or the input should remain (even as a deprecated no-op) for compatibility.
import { Directive, HostBinding, Input, OnDestroy, booleanAttribute } from '@angular/core';
import { IToggleView } from 'igniteui-angular/core';
import { IPositionStrategy, OverlaySettings } from 'igniteui-angular/core';
import { IgxToggleDirective } from '../toggle/toggle.directive';
@Directive()
export abstract class IgxNotificationsDirective extends IgxToggleDirective
implements IToggleView, OnDestroy {
/**
* Sets/gets the `aria-live` attribute.
* If not set, `aria-live` will have value `"polite"`.
*/
@HostBinding('attr.aria-live')
@Input()
public ariaLive = 'polite';
/**
* Sets/gets whether the element will be hidden after the `displayTime` is over.
* Default value is `true`.
*/
@Input({ transform: booleanAttribute })
public autoHide = true;
/**
* Sets/gets the duration of time span (in milliseconds) which the element will be visible
* after it is being shown.
* Default value is `4000`.
*/
@Input()
public displayTime = 4000;
/**
* Controls whether positioning is relative to the viewport or to the nearest positioned container.
*/
@Input()
public positioning: 'viewport' | 'container' = 'viewport';
/**
* Enables/Disables the visibility of the element.
* If not set, the `isVisible` attribute will have value `false`.
*/
@Input({ transform: booleanAttribute })
public get isVisible() {
return !this.collapsed;
}
public set isVisible(value) {
if (value !== this.isVisible) {
if (value) {
requestAnimationFrame(() => {
this.open();
});
} else {
this.close();
}
}
}
/**
* @hidden
* @internal
*/
public textMessage = '';
/**
* @hidden
*/
public timeoutId: number;
/**
* @hidden
*/
protected strategy: IPositionStrategy;
/**
* @hidden
*/
public override open() {
clearInterval(this.timeoutId);
const overlaySettings: OverlaySettings = {
positionStrategy: this.strategy,
closeOnEscape: false,
closeOnOutsideClick: false,
modal: false
};
super.open(overlaySettings);
if (this.autoHide) {
projects/igniteui-angular/grids/core/src/common/grid.interface.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (3)
projects/igniteui-angular/directives/src/directives/toggle/toggle.directive.ts:470
- Removing the
igxToggleOutletinput support here is a public API change (even if it was deprecated) and will break templates using[igxToggleOutlet]. If this removal is intentional, the PR metadata/checklist should mark this as a breaking change and include the appropriate changelog entry and/orng updatemigration guidance for consumers.
export class IgxToggleActionDirective implements OnInit {
protected element = inject(ElementRef);
protected navigationService = inject(IgxNavigationService, { optional: true });
/**
* Provide settings that control the toggle overlay positioning, interaction and scroll behavior.
* ```typescript
* const settings: OverlaySettings = {
* closeOnOutsideClick: false,
* modal: false
* }
* ```
* ---
* ```html
* <!--set-->
* <div igxToggleAction [overlaySettings]="settings"></div>
* ```
*/
@Input()
public overlaySettings: OverlaySettings;
/**
* @hidden
*/
@Input('igxToggleAction')
public set target(target: any) {
if (target !== null && target !== '') {
this._target = target;
}
}
/**
* @hidden
*/
public get target(): any {
if (typeof this._target === 'string') {
return this.navigationService.get(this._target);
}
return this._target;
}
protected _overlayDefaults: OverlaySettings;
protected _target: IToggleView | string;
/**
* @hidden
*/
@HostListener('click')
public onClick() {
const clonedSettings = Object.assign({}, this._overlayDefaults, this.overlaySettings);
this.updateOverlaySettings(clonedSettings);
this.target.toggle(clonedSettings);
}
projects/igniteui-angular/directives/src/directives/notification/notifications.directive.ts:90
IgxNotificationsDirectiveno longer exposes the deprecatedoutletinput and no longer passes it to overlay settings. This is a breaking behavioral/API change for consumers that still rely onoutlet(even if deprecated). If this is intended, please mark the PR as breaking and add migration/changelog notes so downstream apps know how to update.
import { Directive, HostBinding, Input, OnDestroy, booleanAttribute } from '@angular/core';
import { IToggleView } from 'igniteui-angular/core';
import { IPositionStrategy, OverlaySettings } from 'igniteui-angular/core';
import { IgxToggleDirective } from '../toggle/toggle.directive';
@Directive()
export abstract class IgxNotificationsDirective extends IgxToggleDirective
implements IToggleView, OnDestroy {
/**
* Sets/gets the `aria-live` attribute.
* If not set, `aria-live` will have value `"polite"`.
*/
@HostBinding('attr.aria-live')
@Input()
public ariaLive = 'polite';
/**
* Sets/gets whether the element will be hidden after the `displayTime` is over.
* Default value is `true`.
*/
@Input({ transform: booleanAttribute })
public autoHide = true;
/**
* Sets/gets the duration of time span (in milliseconds) which the element will be visible
* after it is being shown.
* Default value is `4000`.
*/
@Input()
public displayTime = 4000;
/**
* Controls whether positioning is relative to the viewport or to the nearest positioned container.
*/
@Input()
public positioning: 'viewport' | 'container' = 'viewport';
/**
* Enables/Disables the visibility of the element.
* If not set, the `isVisible` attribute will have value `false`.
*/
@Input({ transform: booleanAttribute })
public get isVisible() {
return !this.collapsed;
}
public set isVisible(value) {
if (value !== this.isVisible) {
if (value) {
requestAnimationFrame(() => {
this.open();
});
} else {
this.close();
}
}
}
/**
* @hidden
* @internal
*/
public textMessage = '';
/**
* @hidden
*/
public timeoutId: number;
/**
* @hidden
*/
protected strategy: IPositionStrategy;
/**
* @hidden
*/
public override open() {
clearInterval(this.timeoutId);
const overlaySettings: OverlaySettings = {
positionStrategy: this.strategy,
closeOnEscape: false,
closeOnOutsideClick: false,
modal: false
};
super.open(overlaySettings);
if (this.autoHide) {
projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.ts:586
IgxDatePickerComponentno longer applies theoutletinput tooverlaySettingswhen opening (outlet support is effectively removed). Since the file still importsIgxOverlayOutletDirectiveonly for the removed outlet feature, it appears unused now and will trigger@typescript-eslint/no-unused-varswarnings—consider removing it (and any other now-unused outlet-related imports/types) to keep lint clean.
const overlaySettings = Object.assign({}, this.isDropdown
? this.dropDownOverlaySettings
: this.dialogOverlaySettings
, settings);
if (this.isDropdown && this.inputGroupElement) {
overlaySettings.target = this.inputGroupElement;
}
this._overlayId = this._overlayService
.attach(IgxCalendarContainerComponent, this.viewContainerRef, overlaySettings);
this._overlayService.show(this._overlayId);
}
projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.ts
Show resolved
Hide resolved
|
LGTM |
…ove-internal-overlay-outlets
…hub.com/IgniteUI/igniteui-angular into mvenkov/remove-internal-overlay-outlets
damyanpetev
left a comment
There was a problem hiding this comment.
I think some tests still cover funcitoning public outlet props and could be left in place still, but check the coverage report.
Some styling to remove and some nits, but otherwise LGTM
| */ | ||
| @ViewChild('igxLoadingOverlayOutlet', { static: true }) | ||
| public loadingOutlet: ElementRef<HTMLElement>; | ||
| public loadingOverlay?: IgxToggleDirective; |
There was a problem hiding this comment.
Eh, the static flag means it should always be assigned so more of a ! situation, but sure.
| @if (colResizingService.showResizer) { | ||
| <igx-pivot-grid-column-resizer [restrictResizerTop]="calculateResizerTop()"></igx-pivot-grid-column-resizer> | ||
| } | ||
| <div class="igx-grid__loading-outlet" #igxLoadingOverlayOutlet></div> |
There was a problem hiding this comment.
Might want to check other similar deletions (row editing outlet?) don't also leave dangling styles too
| const selectDropdown = Array.from(document.querySelectorAll(`.${QueryBuilderSelectors.DROP_DOWN_LIST_SCROLL}`)) | ||
| .filter(item => (item as HTMLElement).checkVisibility())[index]; |
There was a problem hiding this comment.
Bit odd rework - maybe query the QB element instead of document or pass the QB DebugElement and query the select and then dropdown from that?

Remove any usage of outlet form internal code base.
Closes #17039
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)